blockstore: extract ARC cache from Bloom cache #3026
Conversation
it removes race condition that would happen during various calls License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
| } | ||
| } | ||
| err := b.blockstore.PutMany(bs) | ||
| if err == nil { |
There was a problem hiding this comment.
i'd reverse the logic here to be more conventional:
if err != nil {
return err
}
for ....There was a problem hiding this comment.
Sure, it was refactored out so there was more logic in here.
Kubuxu
force-pushed
the
feature/blockstore-separation
branch
from
dd49a4e
to
a50495d
Compare
|
@Kubuxu theres a lot of failing tests here... |
| } | ||
| cachedbs.Put(b) | ||
|
|
||
| cd.Lock() |
There was a problem hiding this comment.
whats with locking here? you dont need to lock around a variable instantiation
There was a problem hiding this comment.
The race detector was giving me an error otherwise for some reason, I can tinker with it more to find out the exact reason.
Maybe line 47 should be locked instead.
|
what was the actual race condition here that youre fixing? |
|
The race was happening when tests for ARC was running and the bloom cache finished initializing. The test failures you see here are because the gateways were down. |
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feature/blockstore-separation
branch
from
a50495d
to
9543ed6
Compare
| h, ok := b.arc.Get(k) | ||
| if ok { | ||
| return h.(bool), ok | ||
| } else { |
There was a problem hiding this comment.
yeah, it is artefact from the refactor too
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
|
Alright, LGTM. waiting on tests to pass now |
|
All green. |
|
Also the race in question: https://gist.github.com/Kubuxu/575f39058618a3fd5fd3efa518c65c1e |
| blockstore Blockstore | ||
| } | ||
|
|
||
| func arcCached(bs Blockstore, lruSize int) (*arccache, error) { |
it removes race condition that would happen during various calls
If tests are ok, it is good for review.
License: MIT
Signed-off-by: Jakub Sztandera [email protected]